Rollup of 7 pull requests #148553
Closed
Rollup of 7 pull requests #148553
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some sanitizers are part of a system's ABI, like the shadow call stack on Aarch64 and RISC-V Fuchsia. Typically ABI options have other spellings, but LLVM has, for historical reasons, marked this as a sanitizer instead of an alternate ABI option. As a result, Fuchsia targets may not be compiled against the correct ABI unless this option is set. This hasn't caused correctness problems, since the backend reserves the SCS register, and thus preserves its value. But this is an issue for unwinding, as the SCS will not be an array of PCs describing the call complete call chain, and will have gaps from callers that don't use the correct ABI. In the long term, I'd like to see all the sanitizer configs that all frontends copy from clang moved into llvm's libFrontend, and exposed so that frontend consumers can use a small set of simple APIs to use sanitizers in a consistent way across the LLVM ecosystem, but that work is not yet ready today.
This implements a new unstable compiler flag `-Zannotate-moves` that makes
move and copy operations visible in profilers by creating synthetic debug
information. This is achieved with zero runtime cost by manipulating debug
info scopes to make moves/copies appear as calls to `compiler_move<T, SIZE>`
and `compiler_copy<T, SIZE>` marker functions in profiling tools.
This allows developers to identify expensive move/copy operations in their
code using standard profiling tools, without requiring specialized tooling
or runtime instrumentation.
The implementation works at codegen time. When processing MIR operands
(`Operand::Move` and `Operand::Copy`), the codegen creates an `OperandRef`
with an optional `move_annotation` field containing an `Instance` of the
appropriate profiling marker function. When storing the operand,
`store_with_annotation()` wraps the store operation in a synthetic debug
scope that makes it appear inlined from the marker.
Two marker functions (`compiler_move` and `compiler_copy`) are defined
in `library/core/src/profiling.rs`. These are never actually called -
they exist solely as debug info anchors.
Operations are only annotated if the type:
- Meets the size threshold (default: 65 bytes, configurable via
`-Zannotate-moves=SIZE`)
- Has a non-scalar backend representation (scalars use registers,
not memcpy)
This has a very small size impact on object file size. With the default
limit it's well under 0.1%, and even with a very small limit of 8 bytes
it's still ~1.5%. This could be enabled by default.
While looking at the pretty-printers, I found a few minor oddities in StdNonZeroNumberProvider. First, gdb.Type.fields() already returns a sequence, so there's no need to call list(). Second, it's more idiomatic for the (somewhat misnamed) to_string method to simply return the underlying gdb.Value. This also lets gdb apply whatever formats were passed to `print`, as the new test shows. Third, there's no need to use the field's name when looking up a field in a value, the gdb.Field itself can be used.
gdb doesn't have a way to know when an object hasn't yet been
initialized, and in this case, if a pretty-printer returns an absurd
number of children, this can result in apparent hangs in some modes.
This came up specifically with DAP, see this bug report:
https://sourceware.org/bugzilla/show_bug.cgi?id=33594
This patch (mostly) addresses this potential issue in the Rust
pretty-printers, by adding 'num_children' methods. In particular a
method like this is added when the number of children is variable and
also relatively easy to compute. (I.e., I didn't attempt the btree
printers.)
Supplying num_children is good for DAP regardless of the
initialization problem, because DAP requires a count of child objects
and this is more efficient than enumerating the children, which is
gdb's fallback approach.
The link had a stray character that generated an invalid link.
… r=Amanieu Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro closes rust-lang#145649 closes rust-lang#135413 cc: rust-lang#130869 reference PR: rust-lang/reference#1972 # Stabilization report ## Summary This PR stabilizes the following s390x target features: - `vector` - `vector-enhancements-1` - `vector-enhancements-2` - `vector-enhancements-3` - `vector-packed-decimal` - `vector-packed-decimal-enhancement` - `vector-packed-decimal-enhancement-2` - `vector-packed-decimal-enhancement-3` - `nnp-assist` - `miscellaneous-extensions-2` - `miscellaneous-extensions-3` - `miscellaneous-extensions-4` Additionally, it stabilizes the `std::arch::is_s390x_feature_detected!` macro itself and stably accepts the target features listed above. ## Tests & ABI details Only the `vector` target feature changes the ABI, much like e.g. `avx2` it will, depending on the ABI, pass vector types in vector registers. This behavior is tested extensively: - [tests/assembly-llvm/s390x-vector-abi.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/codegen-llvm/s390x-simd.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/assembly-llvm/s390x-vector-abi.rs) - [tests/ui/abi/simd-abi-checks-s390x.rs ](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs ) The remaining features don't influence the ABI, they only influence instruction selection. In stdarch we test that the expected instructions are in fact generated when the target feature is enabled. ## Implementation history For `is_s390x_feature_detected!`: - rust-lang/stdarch#1699 - rust-lang#138275 - rust-lang/stdarch#1720 - rust-lang/stdarch#1832 For `vector` and friends - rust-lang#127506 - rust-lang#135630 - rust-lang#141250 ## Unresolved questions There is a fixme in [tests/ui/abi/simd-abi-checks-s390x.rs](https://github.com/rust-lang/rust/blob/22a86f8280becb12c34ee3efd952baf5cf086fa0/tests/ui/abi/simd-abi-checks-s390x.rs): ``` // FIXME: +soft-float itself doesn't set -vector //`@[z13_soft_float]` compile-flags: --target s390x-unknown-linux-gnu -C target-cpu=z13 -C target-feature=-vector,+soft-float //`@[z13_soft_float]` needs-llvm-components: systemz ``` I'm not sure whether that blocks stabilization? --- The implementation first extracts the listed target features into their own `s390x_target_feature_vector` rust feature, and then stabilizes that. best reviewed commit-by-commit r? `@Amanieu` cc `@uweigand` `@taiki-e`
…ochenkov Add default sanitizers to TargetOptions Some sanitizers are part of a system's ABI, like the shadow call stack on Aarch64 and RISC-V Fuchsia. Typically ABI options have other spellings, but LLVM has, for historical reasons, marked this as a sanitizer instead of an alternate ABI option. As a result, Fuchsia targets may not be compiled against the correct ABI unless this option is set. This hasn't caused correctness problems, since the backend reserves the SCS register, and thus preserves its value. But this is an issue for unwinding, as the SCS will not be an array of PCs describing the call complete call chain, and will have gaps from callers that don't use the correct ABI. In the long term, I'd like to see all the sanitizer configs that all frontends copy from clang moved into llvm's libFrontend, and exposed so that frontend consumers can use a small set of simple APIs to use sanitizers in a consistent way across the LLVM ecosystem, but that work is not yet ready today.
…saethlin Add -Zannotate-moves for profiler visibility of move/copy operations (codegen) **Note:** this is an alternative implementation of rust-lang#147206; rather than being a MIR transform, it adds the annotations closer to codegen. It's functionally the same but the implementation is lower impact and it could be more correct. --- This implements a new unstable compiler flag `-Zannotate-moves` that makes move and copy operations visible in profilers by creating synthetic debug information. This is achieved with zero runtime cost by manipulating debug info scopes to make moves/copies appear as calls to `compiler_move<T, SIZE>` and `compiler_copy<T, SIZE>` marker functions in profiling tools. This allows developers to identify expensive move/copy operations in their code using standard profiling tools, without requiring specialized tooling or runtime instrumentation. The implementation works at codegen time. When processing MIR operands (`Operand::Move` and `Operand::Copy`), the codegen creates an `OperandRef` with an optional `move_annotation` field containing an `Instance` of the appropriate profiling marker function. When storing the operand, `store_with_annotation()` wraps the store operation in a synthetic debug scope that makes it appear inlined from the marker. Two marker functions (`compiler_move` and `compiler_copy`) are defined in `library/core/src/profiling.rs`. These are never actually called - they exist solely as debug info anchors. Operations are only annotated if: - We're generating debug info and the feature is enabled. - Meets the size threshold (default: 65 bytes, configurable via `-Zannotate-moves=SIZE`), and is non-zero - Has a memory representation This has a very small size impact on object file size. With the default limit it's well under 0.1%, and even with a very small limit of 8 bytes it's still ~1.5%. This could be enabled by default.
…ror-handling, r=lolbinarycat [rustdoc] Gracefully handle error in case we cannot run the compiler in doctests Fixes bug reported in [this comment](rust-lang#102981 (comment)). r? `@lolbinarycat`
Minor fixes to StdNonZeroNumberProvider for gdb While looking at the pretty-printers, I found a few minor oddities in StdNonZeroNumberProvider. First, gdb.Type.fields() already returns a sequence, so there's no need to call list(). Second, it's more idiomatic for the (somewhat misnamed) to_string method to simply return the underlying gdb.Value. This also lets gdb apply whatever formats were passed to `print`, as the new test shows. Third, there's no need to use the field's name when looking up a field in a value, the gdb.Field itself can be used.
Add num_children method to some gdb pretty-printers
gdb doesn't have a way to know when an object hasn't yet been initialized, and in this case, if a pretty-printer returns an absurd number of children, this can result in apparent hangs in some modes. This came up specifically with DAP, see this bug report:
https://sourceware.org/bugzilla/show_bug.cgi?id=33594
This patch (mostly) addresses this potential issue in the Rust pretty-printers, by adding 'num_children' methods. In particular a method like this is added when the number of children is variable and also relatively easy to compute. (I.e., I didn't attempt the btree printers.)
Supplying num_children is good for DAP regardless of the initialization problem, because DAP requires a count of child objects and this is more efficient than enumerating the children, which is gdb's fallback approach.
Fix broken qemu-cskyv2 link The link had a stray character that generated an invalid link.
rustbot
added
A-LLVM
Member
Author
|
Rollup of everything. @bors r+ rollup=never p=5 |
Collaborator
bors
added
S-waiting-on-bors
Collaborator
bors
added a commit
that referenced
this pull request
Nov 6, 2025
Rollup of 7 pull requests Successful merges: - #145656 (Stabilize s390x `vector` target feature and `is_s390x_feature_detected!` macro) - #147043 (Add default sanitizers to TargetOptions) - #147803 (Add -Zannotate-moves for profiler visibility of move/copy operations (codegen)) - #147912 ([rustdoc] Gracefully handle error in case we cannot run the compiler in doctests) - #148540 (Minor fixes to StdNonZeroNumberProvider for gdb) - #148541 (Add num_children method to some gdb pretty-printers) - #148549 (Fix broken qemu-cskyv2 link) Failed merges: - #147586 (std-detect: improve detect macro docs) r? `@ghost` `@rustbot` modify labels: rollup
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Collaborator
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
rustbot
removed
the
S-waiting-on-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
vectortarget feature andis_s390x_feature_detected!macro #145656 (Stabilize s390xvectortarget feature andis_s390x_feature_detected!macro)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup